Reload app settings on .env file change for local deployment#223
Reload app settings on .env file change for local deployment#223igorbenav merged 2 commits intobenavlabs:stagingfrom
Conversation
10d49aa to
f1134f6
Compare
There was a problem hiding this comment.
I'm testing this pr, and you said "While doing that then it is also possible to enable reloading of the application based on changes to the .env file, which is quite useful for development" but it doesn't seem to happen. I tried changing the ADMIN_PASSWORD on .env with the app running and then login as the admin with the new password.
It says the new admin tables are being created, says the app reloaded but the password doesn't actually change
f1134f6 to
fd11216
Compare
…and in the dockerfile
|
@LucasQR thanks so much for checking, perhaps you forgot to add BTW because the commits were squashed from the other PR I had to force pushed, apologies but you will need to force pull. Hope not too much trouble. let me know if you still see it not reloading on .env changes. Thanks again. |
|
Hi @LucasQR can you have another look? Check my comment above, maybe that was the issue |
|
I'll take a look at it right now, but after I went after the reason for that error I started to think it was caused by a quirk with the logging on crudadmin. Anyway I'll check it and get back to you as soon as possible. And just as a reminder, tag me or add me as a reviewer on any following pr on benav labs |
|
Update, it is updating the app name, description, etc(anything that doesn't involve db) . I checked further ( and may raise this as an issue after talking to @igorbenav about it) but the problem I had was caused by a silent error on crudadmin's I'd recommend putting some sort of warning or raising an error if the user tries to change db realated things on .env (aswell as updating the docs to reflect the changes made by these prs) but I'll aprove it even without |
LucasQR
left a comment
There was a problem hiding this comment.
works properly although i'd like a bit of documentation on this
|
Ok, thanks @LucasQR |
|
Sure, the quirk i talked about is on crudadmin, not on the boilerplate so you can just keep going without fixing it. I'll review the other ones now |
|
@rragundez I'll create a staging branch, let's do all the prs there then. I'll take over reviews here |
At the moment there is a mixture of getting settings from environment variables and from an .env file. This is confusing and can lead to cumbersome bugs. This PR addresses the problem only for the local deployment (prod and staging will come later) where it makes it clear that settings will only be taken from the .env file (via environment variables) and removes the setting of environment variables to the runtime via the docker compose definition.
While doing that then it is also possible to enable reloading of the application based on changes to the .env file, which is quite useful for development. At the moment the app only reload when py files change.
It also removed the command in the docker compose definition and simplifies the responsibility of running the command by giving it solely to the Dockerfile (at the moment it seems to be and be promoted via the comments to change the command in either dockerfile or docker compose which is confusing for a user).